Call rebalance API once all pods are updated #625
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
HoustonPutman
requested changes
Oct 2, 2023
There was a problem hiding this comment.
This is great! I have two edge cases that I think we need to account for:
- In the case that all the pods are updates, the async rebalance command will be started. The user can then change the spec again, and the updatedPodCount will then again be
0, because all pods will be out of date. At that point we will have forgotten about the rebalance command that we started. We need to make sure that we complete any async command before we start a new one, so I suggest that we split off the async command to a separate ClusterOp, that we start after the restart is complete. That way another restart will only begin once the rebalance is finished. - If I'm correct, we are rebalancing even if replicas weren't moved around during the rolling restart. In my opinion, we should only rebalance if replicas were moved around.
HoustonPutman
approved these changes
Oct 9, 2023
There was a problem hiding this comment.
I think this is good to go @radu-gheorghe , give it a look!
|
Looks good to me, too, thanks a lot @HoustonPutman! I already had a look yesterday, too, and noticed you couldn't help yourself and fixed your 2) above 🙂 |
Haha I remembered that we had the lovely readinessConditions that told us if the cloud was ephemeral, so it was very easy to do it! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Timid attempt to fix #615
It works on my machine, it seems to fix the problem, but:
Any feedback is welcome, I'd be glad to push this forward!